fix(event_handler): parse single list items in form data #7415
Conversation
pull-request-size
bot
added
the
size/S
tonnico
force-pushed
the
feat/bug-7342
branch
from
c61dbe8 to
9c056fe
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7415 +/- ##
===========================================
+ Coverage 96.37% 96.42% +0.04%
===========================================
Files 275 275
Lines 13048 13055 +7
Branches 974 974
===========================================
+ Hits 12575 12588 +13
+ Misses 366 362 -4
+ Partials 107 105 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @tonnico, thank you for the PR! The static analysis shows a warning on the function touched by the changes. With the new additions, the cognitive complexity of the function went up because of the excessive branching logic. Would it be possible to refactor it and reduce it? Thank you. |
There was a problem hiding this comment.
See my comment above about reducing cognitive complexity and address the SonarCloud issue.
Let us know if you have any questions.
There was a problem hiding this comment.
Hey @tonnico thanks a lot for sending this PR. What do you think about adding a test with a field that is annotated as a list but contains only a single value when submitted?! Idk if we have this test and this was the cause of this issue.
Pls let me know what do you think.
pull-request-size
bot
added
size/L
tests/functional/event_handler/_pydantic/test_openapi_validation_middleware.py
Show resolved
Hide resolved
I refactored the function. |
dreamorosi
dismissed
their stale review
With the CI being green and SonarCloud clear, I'm leaving the final review to @leandrodamascena.
dreamorosi
requested review from
leandrodamascena
and removed request for
dreamorosi and
leandrodamascena
|
Hi @tonnico, thank you for addressing the review feedback - @leandrodamascena will review this tomorrow or Friday at the latest. Thank you for your patience and contribution! |
leandrodamascena
changed the title
|
I'm reviewing it now! I see some errors in CI related to the glide library, probably because I merged a PR today morning. Let me see what's going on. |
|
There was a problem hiding this comment.
Hi @tonnico! Sorry for the slight delay in reviewing, but this PR is really awesome! Nothing to change, thank you very much for reducing the code complexity by moving some of the logic into separate functions.
APPROVED!
|
@tonnico - thank you for fixing this - much appreciated! |
4 participants
Issue number: closes #7342
Summary
Changes
_parse_form_dataUser experience
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.